-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Speed up drop database #6623
Speed up drop database #6623
Conversation
By analyzing the blame information on this pull request, we identified @e-dard to be a potential reviewer |
Drop database was closing and deleting each shard dir individually and serially. It would then delete the empty database dirs. This changes drop database to close all shards in parallel and run one os.RemoveAll to remove everything under the db dir which is more efficient. This also reworked the locking to avoid locking the tsdb.Store for long periods of time. That can cause queries and writes for other databases to block as well.
} | ||
} | ||
s.mu.RUnlock() | ||
wg.Wait() | ||
close(responses) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could further improve this by getting rid of the WaitGroup
and counting how many responses you have received in your for
loop further down. Once you have len(s.shards)
responses you can close the channel and exit the loop.
This has the added benefit that you don't have to wait for all the shards to attempt to close themselves, before finding out that one of them early on errored out anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, thinking about it I guess you would probably want to carry on deleting the shards after an error anyway right? So then the waitgroup does make sense..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Wanted to have them all try and either complete or fail before returning an error.
+1 |
Required for all non-trivial PRs
Drop database was closing and deleting each shard dir individually and
serially. It would then delete the empty database dirs.
This changes drop database to close all shards in parallel and run
one
os.RemoveAll
to remove everything under the db dir which is moreefficient.
This also reworked the locking to avoid locking the
tsdb.Store
forlong periods of time. That can cause queries and writes for other
databases to block as well.